Skip to content

Conversation

romancardenas
Copy link
Contributor

This PR ports some of the ongoing work in the RISC-V ecosystem to Cortex-M targets. Namely, I adapted the riscv-pac crate to the new cortex-m-pac crate.

The main purpose of this crate is to isolate fundamental traits and data types from potential breaking changes in cortex-m. Namely, it currently contains traits for enumerating exceptions, interrupts, priorities, and core IDs. It also provides a basic fallible function support for Cortex-M devices, with an error enum with some of the most typical issues developers may face.

Related PRs in RISC-V: #222 and #223

Related PRs in Cortex-M: #488

Once this is merged, I plan to continue porting new functionalities from RISC-V to Cortex-M (e.g., new syntax for interrupt and exception macros)

@romancardenas romancardenas requested a review from a team as a code owner October 9, 2024 21:11
@romancardenas
Copy link
Contributor Author

Implementing core::error::Error implies bumping MSRV to 1.81. Let me know what you think and if it is worth it such a change

@jannic
Copy link
Member

jannic commented Oct 18, 2024

CI failed because the ubuntu-latest label migrated to ubuntu 24.04 (https://github.blog/changelog/2024-09-25-actions-new-images-and-ubuntu-latest-changes/), and it looks like there the qemu package is no longer available.

A quick fix would be to change the runner image to ubuntu-22.04.

Alternatively, CI could use qemu-system-x86-64 instead of qemu. However this might need some tweaking of command lines.

@jannic
Copy link
Member

jannic commented Oct 18, 2024

Alternatively, CI could use qemu-system-x86-64 instead of qemu. However this might need some tweaking of command lines.

That was easier than I had expected: #562

@romancardenas
Copy link
Contributor Author

Rebased and added #[non_exhaustive] to Error

@thalesfragoso
Copy link
Member

CC @adamgreig Since this is replacing a previous PR from you.

@romancardenas
Copy link
Contributor Author

Hey @adamgreig could you take a quick look at this? I am planning to include a few suggestions from this PR to riscv, but I'd love to get your input before

@adamgreig
Copy link
Member

I'm very happy with the InterruptNumber trait and I think the additional traits are very sensible extensions too, no comments on them and definitely something I want before we release cortex-m 0.8.

I'm not entirely sure of the utility of the new Error enum apart from giving us something to return from these traits. It otherwise feels quite generic and not really "cortex-m"-specific at all. I wonder if it should be tailored to be just the "error returned from these pac traits" instead of trying to be a more generically useful error. It reminds me a bit of having things like ValueError and KeyError in Python which are generally quite useful but not such a thing in Rust afaik.

Finally I wonder if the name should be "cortex-m-pac-traits" instead of "cortex-m-pac". I always imagined "cortex-m-pac" would be a PAC for Cortex-M core peripherals/registers, using the same generator as other PACs to replace the current ad-hoc register access we have in the cortex-m crate.

@romancardenas
Copy link
Contributor Author

I'm not entirely sure of the utility of the new Error enum apart from giving us something to return from these traits.

In RISC-V, we use this Error enum in try_* functions within the riscv and riscv-rt crates. Basically, CSRs include fallible methods that may return errors and infallible methods that unwrap() their fallible counterpart. Currently, riscv-pac has the exact same error types as the ones in this PR. We could move them to a more generic place and let cortex-m and riscv rely on a common crate, but maybe that would lead to more difficult ways of keeping both ISAs up to date.

Finally I wonder if the name should be "cortex-m-pac-traits" instead of "cortex-m-pac"

I chose cortex-m-pac to be aligned with current riscv-pac. This crate contains useful abstractions to be implemented in PACs. It does not only include traits, but also this generic Error enum that can be used in CSRs (both standard and vendor-specific) or standard peripheral interfaces. In any case, the naming can be modified.

@rursprung
Copy link

Finally I wonder if the name should be "cortex-m-pac-traits" instead of "cortex-m-pac"

the same could be argued for embedded-hal which almost exclusively contains traits. so leaving -traits away keeps them aligned; on the other hand you could argue that you have a chance to do it right here (if you consider adding -traits as "right"; i have no firm opinion on this)

@adamgreig
Copy link
Member

Error

Got it, so the utility is sharing it between riscv and riscv-rt? Are they used outside of that?

cortex-m-pac

I get that, but it's still not itself a PAC, and everything else named -pac is a PAC, including the cortex-m-pac PAC I'd imagined having for doing CSR access on cortex-m core peripherals. I guess that's all in riscv along with the equivalent "HAL" higher-level methods?

@romancardenas
Copy link
Contributor Author

Yes, riscv and riscv-rt are very similar to cortex-m and cortex-m-rt. In riscv-pac we isolate foundational abstractions such as the interrupt, exception, etc. traits and an error enumeration that is used for the CSR and standard peripheral APIs. In this way, bumping the version in riscv or riscv-rt does not break everything.

The RISC-V PACs I know do not include the suffixpac (e.g., e310x). Do you prefer maybe cortex-m-impl? or cortex-m-common?

@jonathanpallant
Copy link
Contributor

I'd vote for calling this cortex-m-types. It's too late to rename embedded-hal but we can make this one clearer up front now, before it has any users.

  • chip-pac depends on arch-types
  • arch-rt depends on arch-types
  • arch depends on arch-types
  • app depends on chip-pac and arch-rt (and maybe arch)

I might make the same split for cortex-ar.

@romancardenas
Copy link
Contributor Author

romancardenas commented Oct 1, 2025

arch-types sounds good to me. We can adopt the naming in RISC-V, if needed

@romancardenas romancardenas changed the title cortex-m-pac crate: standard traits and result handling for Cortex-M targets cortex-m-types crate: standard traits and result handling for Cortex-M targets Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants